Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

String literal rename support #39298

Merged
merged 13 commits into from
Nov 6, 2020
Merged

String literal rename support #39298

merged 13 commits into from
Nov 6, 2020
+77 −7

Conversation

Kingwl
Copy link
Contributor

@Kingwl Kingwl commented Jun 27, 2020

Fixes #5602

@Kingwl Kingwl changed the title String literal refactor String literal rename support Jun 27, 2020
@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 28, 2020

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 28, 2020 •
edited
Loading

Heya @Kingwl, I've started to run the tarball bundle task on this PR at 801edb2. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 28, 2020 •
edited
Loading

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/78044/artifacts?artifactName=tgz&fileId=295685A5BF3AAA029459E0812C1B6ADAFB9F02FCB950BC2A88A0BB0B0D902BA302&fileName=/typescript-4.0.0-insiders.20200628.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Jul 23, 2020
@Kingwl
Copy link
Contributor Author

Kingwl commented Jul 28, 2020

@typescript-bot test this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jul 28, 2020 •
edited
Loading

Heya @Kingwl, I've started to run the extended test suite on this PR at 5a937eb. You can monitor the build here.

@Kingwl
Copy link
Contributor Author

Kingwl commented Aug 3, 2020

UP 👆

andrewbranch
andrewbranch reviewed Sep 2, 2020
View reviewed changes
src/services/utilities.ts Outdated Show resolved Hide resolved
src/services/utilities.ts Outdated Show resolved Hide resolved
@andrewbranch
Copy link
Member

@Kingwl have you checked out what happened with the tests? If you can fix those soon, I think we can get this in before 4.1 beta.

@Kingwl
Copy link
Contributor Author

Kingwl commented Sep 14, 2020

Sorry for the delay. I'm on vacation this weekend.
Hope nothing missed。

andrewbranch
andrewbranch approved these changes Sep 22, 2020
View reviewed changes
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Kingwl! Since this is really a feature and we’re past the 4.1 beta, I think we’re going to hold it for 4.2. I’ll come back to merge as soon as we cut the release branch.

src/services/findAllReferences.ts Outdated Show resolved Hide resolved
src/services/findAllReferences.ts Outdated Show resolved Hide resolved
src/services/findAllReferences.ts Outdated Show resolved Hide resolved
src/services/findAllReferences.ts Outdated Show resolved Hide resolved
src/services/findAllReferences.ts Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
codart1
codart1 reviewed Oct 17, 2020
View reviewed changes
tests/cases/fourslash/renameStringLiteralOk1.ts
}
const d: 'foo' = 'foo'
declare const f: Foo
f.f = 'foo'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
f.f = 'foo'
f.f = 'foo'

add new line

@andrewbranch andrewbranch merged commit ca7510e into microsoft:master Nov 6, 2020
@Kingwl Kingwl deleted the string-literal-refactor branch November 6, 2020 18:11
@aspirisen
Copy link

Hi, is it known issue that in such cases renaming doesn't work

type Actions = { type: 'one', payload: number } | { type: 'two', payload: boolean }
const q: Actions = { type: 'one', payload: 1 }

Checked on ts=4.2.0-dev.20201109
Playground

@Kingwl
Copy link
Contributor Author

Kingwl commented Nov 11, 2020

Well. Even the current implement has many limitation. But I don't think That is expected.

Could you create an issue to track that please?
I'm also happy do it that if you are not interested at it.

@aspirisen aspirisen mentioned this pull request Nov 11, 2020
Open
@wclr
Copy link

wclr commented Dec 10, 2020

@Kingwl
Oh, this will be really great when the common case for redux/ect actions will be supported.

This was referenced Dec 10, 2020
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codart1 codart1 codart1 left review comments

@andrewbranch andrewbranch andrewbranch approved these changes

Assignees

@andrewbranch andrewbranch

Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

String Literal refactoring support
6 participants
@Kingwl @typescript-bot @andrewbranch @aspirisen @wclr @codart1